From c5a7c9eea9b8dd7d54ffb826ac1b7cf43ad501ea Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Mon, 5 Jan 2026 20:36:07 -0300 Subject: [PATCH] [PATCH] permission: include permission check on lib/fs/promises PR-URL: https://github.com/nodejs-private/node-private/pull/840 CVE-ID: CVE-2026-21716 Gbp-Pq: Topic sec Gbp-Pq: Name 53-include-permission-check-on-lib-fs-promises.patch --- lib/internal/fs/promises.js | 13 ++ src/node_file-inl.h | 33 +++-- src/node_file.cc | 2 - test/fixtures/permission/fs-read.js | 200 +++++++++++++++++++++++++ test/fixtures/permission/fs-write.js | 211 ++++++++++++++++++++++++++- 5 files changed, 438 insertions(+), 21 deletions(-) diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index d96584bca..5823881c2 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -17,6 +17,7 @@ const { Symbol, Uint8Array, FunctionPrototypeBind, + uncurryThis, } = primordials; const { fs: constants } = internalBinding('constants'); @@ -30,6 +31,8 @@ const { const binding = internalBinding('fs'); const { Buffer } = require('buffer'); +const { isBuffer: BufferIsBuffer } = Buffer; +const BufferToString = uncurryThis(Buffer.prototype.toString); const { codes: { @@ -1012,6 +1015,10 @@ async function fstat(handle, options = { bigint: false }) { async function lstat(path, options = { bigint: false }) { path = getValidatedPath(path); + if (permission.isEnabled() && !permission.has('fs.read', path)) { + const resource = pathModule.toNamespacedPath(BufferIsBuffer(path) ? BufferToString(path) : path); + throw new ERR_ACCESS_DENIED('Access to this API has been restricted', 'FileSystemRead', resource); + } const result = await PromisePrototypeThen( binding.lstat(pathModule.toNamespacedPath(path), options.bigint, kUsePromises), @@ -1065,6 +1072,9 @@ async function unlink(path) { } async function fchmod(handle, mode) { + if (permission.isEnabled()) { + throw new ERR_ACCESS_DENIED('fchmod API is disabled when Permission Model is enabled.'); + } mode = parseFileMode(mode, 'mode'); return await PromisePrototypeThen( binding.fchmod(handle.fd, mode, kUsePromises), @@ -1105,6 +1115,9 @@ async function lchown(path, uid, gid) { async function fchown(handle, uid, gid) { validateInteger(uid, 'uid', -1, kMaxUserId); validateInteger(gid, 'gid', -1, kMaxUserId); + if (permission.isEnabled()) { + throw new ERR_ACCESS_DENIED('fchown API is disabled when Permission Model is enabled.'); + } return await PromisePrototypeThen( binding.fchown(handle.fd, uid, gid, kUsePromises), undefined, diff --git a/src/node_file-inl.h b/src/node_file-inl.h index 36c2f8067..cc7ed2166 100644 --- a/src/node_file-inl.h +++ b/src/node_file-inl.h @@ -287,21 +287,27 @@ FSReqBase* GetReqWrap(const v8::FunctionCallbackInfo& args, int index, bool use_bigint) { v8::Local value = args[index]; + FSReqBase* result = nullptr; if (value->IsObject()) { - return Unwrap(value.As()); - } - - Realm* realm = Realm::GetCurrent(args); - BindingData* binding_data = realm->GetBindingData(); - - if (value->StrictEquals(realm->isolate_data()->fs_use_promises_symbol())) { - if (use_bigint) { - return FSReqPromise::New(binding_data, use_bigint); - } else { - return FSReqPromise::New(binding_data, use_bigint); + result = Unwrap(value.As()); + } else { + Realm* realm = Realm::GetCurrent(args); + BindingData* binding_data = realm->GetBindingData(); + + if (value->StrictEquals(realm->isolate_data()->fs_use_promises_symbol())) { + if (use_bigint) { + result = + FSReqPromise::New(binding_data, use_bigint); + } else { + result = + FSReqPromise::New(binding_data, use_bigint); + } } } - return nullptr; + if (result != nullptr) { + result->SetReturnValue(args); + } + return result; } // Returns nullptr if the operation fails from the start. @@ -320,10 +326,7 @@ FSReqBase* AsyncDestCall(Environment* env, FSReqBase* req_wrap, uv_req->path = nullptr; after(uv_req); // after may delete req_wrap if there is an error req_wrap = nullptr; - } else { - req_wrap->SetReturnValue(args); } - return req_wrap; } diff --git a/src/node_file.cc b/src/node_file.cc index 03c3b20b5..bdfcb6e46 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -2418,8 +2418,6 @@ static void WriteString(const FunctionCallbackInfo& args) { uv_req->path = nullptr; AfterInteger(uv_req); // after may delete req_wrap_async if there is // an error - } else { - req_wrap_async->SetReturnValue(args); } } else { // write(fd, string, pos, enc, undefined, ctx) CHECK_EQ(argc, 6); diff --git a/test/fixtures/permission/fs-read.js b/test/fixtures/permission/fs-read.js index 03261d975..fb4039440 100644 --- a/test/fixtures/permission/fs-read.js +++ b/test/fixtures/permission/fs-read.js @@ -4,6 +4,8 @@ const common = require('../../common'); const assert = require('assert'); const fs = require('fs'); +const fsPromises = require('node:fs/promises'); + const path = require('path'); const blockedFile = process.env.BLOCKEDFILE; @@ -446,6 +448,204 @@ const regularFile = __filename; })); } +// fsPromises.readFile +{ + assert.rejects(async () => { + await fsPromises.readFile(blockedFile); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.readFile(blockedFileURL); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); +} + +// fsPromises.stat +{ + assert.rejects(async () => { + await fsPromises.stat(blockedFile); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.stat(blockedFileURL); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.stat(path.join(blockedFolder, 'anyfile')); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(path.join(blockedFolder, 'anyfile')), + })).then(common.mustCall()); +} + +// fsPromises.access +{ + assert.rejects(async () => { + await fsPromises.access(blockedFile, fs.constants.R_OK); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.access(blockedFileURL, fs.constants.R_OK); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.access(path.join(blockedFolder, 'anyfile'), fs.constants.R_OK); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(path.join(blockedFolder, 'anyfile')), + })).then(common.mustCall()); +} + +// fsPromises.copyFile +{ + assert.rejects(async () => { + await fsPromises.copyFile(blockedFile, path.join(blockedFolder, 'any-other-file')); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.copyFile(blockedFileURL, path.join(blockedFolder, 'any-other-file')); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); +} + +// fsPromises.cp +{ + assert.rejects(async () => { + await fsPromises.cp(blockedFile, path.join(blockedFolder, 'any-other-file')); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.cp(blockedFileURL, path.join(blockedFolder, 'any-other-file')); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); +} + +// fsPromises.open +{ + assert.rejects(async () => { + await fsPromises.open(blockedFile, 'r'); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.open(blockedFileURL, 'r'); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.open(path.join(blockedFolder, 'anyfile'), 'r'); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(path.join(blockedFolder, 'anyfile')), + })).then(common.mustCall()); +} + +// fsPromises.opendir +{ + assert.rejects(async () => { + await fsPromises.opendir(blockedFolder); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFolder), + })).then(common.mustCall()); +} + +// fsPromises.readdir +{ + assert.rejects(async () => { + await fsPromises.readdir(blockedFolder); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFolder), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.readdir(blockedFolder, { recursive: true }); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFolder), + })).then(common.mustCall()); +} + +// fsPromises.rename +{ + assert.rejects(async () => { + await fsPromises.rename(blockedFile, 'newfile'); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.rename(blockedFileURL, 'newfile'); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); +} + +// fsPromises.lstat +{ + assert.rejects(async () => { + await fsPromises.lstat(blockedFile); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.lstat(blockedFileURL); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.lstat(path.join(blockedFolder, 'anyfile')); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + })).then(common.mustCall()); +} + // fs.lstat { assert.throws(() => { diff --git a/test/fixtures/permission/fs-write.js b/test/fixtures/permission/fs-write.js index 5461a21aa..eac6004ed 100644 --- a/test/fixtures/permission/fs-write.js +++ b/test/fixtures/permission/fs-write.js @@ -5,6 +5,7 @@ common.skipIfWorker(); const assert = require('assert'); const fs = require('fs'); +const fsPromises = require('node:fs/promises'); const path = require('path'); const regularFolder = process.env.ALLOWEDFOLDER; @@ -197,6 +198,13 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER; code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', })); + + assert.rejects(async () => { + await fsPromises.mkdtemp(path.join(blockedFolder, 'any-folder')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + }); } // fs.rename @@ -330,7 +338,7 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER; permission: 'FileSystemWrite', })); assert.rejects(async () => { - await fs.promises.open(blockedFile, fs.constants.O_RDWR | fs.constants.O_NOFOLLOW); + await fsPromises.open(blockedFile, fs.constants.O_RDWR | fs.constants.O_NOFOLLOW); }, { code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', @@ -369,7 +377,7 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER; permission: 'FileSystemWrite', }); assert.rejects(async () => { - await fs.promises.chmod(blockedFile, 0o755); + await fsPromises.chmod(blockedFile, 0o755); }, { code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', @@ -384,7 +392,7 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER; permission: 'FileSystemWrite', })); assert.rejects(async () => { - await fs.promises.lchmod(blockedFile, 0o755); + await fsPromises.lchmod(blockedFile, 0o755); }, { code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', @@ -409,7 +417,7 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER; permission: 'FileSystemWrite', }); assert.rejects(async () => { - await fs.promises.appendFile(blockedFile, 'new data'); + await fsPromises.appendFile(blockedFile, 'new data'); }, { code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', @@ -598,4 +606,199 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER; }, { code: 'ERR_ACCESS_DENIED', }); +} + +// fsPromises.writeFile +{ + assert.rejects(async () => { + await fsPromises.writeFile(blockedFile, 'example'); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.writeFile(blockedFileURL, 'example'); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.writeFile(path.join(blockedFolder, 'anyfile'), 'example'); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(blockedFolder, 'anyfile')), + }).then(common.mustCall()); +} + +// fsPromises.utimes +{ + assert.rejects(async () => { + await fsPromises.utimes(blockedFile, new Date(), new Date()); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.utimes(blockedFileURL, new Date(), new Date()); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.utimes(path.join(blockedFolder, 'anyfile'), new Date(), new Date()); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(blockedFolder, 'anyfile')), + }).then(common.mustCall()); +} + +// fsPromises.lutimes +{ + assert.rejects(async () => { + await fsPromises.lutimes(blockedFile, new Date(), new Date()); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.lutimes(blockedFileURL, new Date(), new Date()); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); +} + +// fsPromises.mkdir +{ + assert.rejects(async () => { + await fsPromises.mkdir(path.join(blockedFolder, 'any-folder')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(blockedFolder, 'any-folder')), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.mkdir(path.join(relativeProtectedFolder, 'any-folder')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(relativeProtectedFolder, 'any-folder')), + }).then(common.mustCall()); +} + +// fsPromises.rename +{ + assert.rejects(async () => { + await fsPromises.rename(blockedFile, path.join(blockedFile, 'renamed')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.rename(blockedFileURL, path.join(blockedFile, 'renamed')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.rename(regularFile, path.join(blockedFolder, 'renamed')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(blockedFolder, 'renamed')), + }).then(common.mustCall()); +} + +// fsPromises.copyFile +{ + assert.rejects(async () => { + await fsPromises.copyFile(regularFile, path.join(blockedFolder, 'any-file')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(blockedFolder, 'any-file')), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.copyFile(regularFile, path.join(relativeProtectedFolder, 'any-file')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(relativeProtectedFolder, 'any-file')), + }).then(common.mustCall()); +} + +// fsPromises.cp +{ + assert.rejects(async () => { + await fsPromises.cp(regularFile, path.join(blockedFolder, 'any-file')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(blockedFolder, 'any-file')), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.cp(regularFile, path.join(relativeProtectedFolder, 'any-file')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(relativeProtectedFolder, 'any-file')), + }).then(common.mustCall()); +} + +// fsPromises.unlink +{ + assert.rejects(async () => { + await fsPromises.unlink(blockedFile); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.unlink(blockedFileURL); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); +} + +// FileHandle.chmod (fchmod) with read-only fd +{ + assert.rejects(async () => { + // blocked file is allowed to read + const fh = await fsPromises.open(blockedFile, 'r'); + try { + await fh.chmod(0o777); + } finally { + await fh.close(); + } + }, { + code: 'ERR_ACCESS_DENIED', + }).then(common.mustCall()); +} + +// FileHandle.chown (fchown) with read-only fd +{ + assert.rejects(async () => { + // blocked file is allowed to read + const fh = await fsPromises.open(blockedFile, 'r'); + try { + await fh.chown(999, 999); + } finally { + await fh.close(); + } + }, { + code: 'ERR_ACCESS_DENIED', + }).then(common.mustCall()); } \ No newline at end of file -- 2.30.2